feat(a2asrv): add AuditLogger for structured A2A call auditing - #379
feat(a2asrv): add AuditLogger for structured A2A call auditing#379kuangmi-bit wants to merge 6 commits into
Conversation
Add a new a2asrv/costlimiter package that provides cost-based budget enforcement as a CallInterceptor. This complements the existing ConcurrencyConfig (which limits in-flight executions) with cost-aware budgeting — useful for LLM token budgets, compute-time quotas, or monetary credit limits. Key components: - CostLimiter: a server-side CallInterceptor that estimates request cost via a pluggable CostFunc and reserves budget via a CostStore - CostStore interface: Available/Reserve/Release with atomic semantics - InMemoryCostStore: thread-safe in-memory implementation - ScopeFunc: per-tenant/per-user budget scoping via CallContext - Cancelled requests release reserved budget back to the pool All 8 tests pass. The package has no external dependencies beyond a2asrv.
Add a new a2asrv/auditlogger package that records structured audit events for every A2A protocol method invocation as a CallInterceptor. The audit event schema aligns with IBM SMF Type 110 (CICS transaction audit): who called what, when, with what result, and duration. Key components: - AuditLogger: server-side CallInterceptor, records Before→After timing - AuditWriter interface: pluggable backend for audit event persistence - InMemoryWriter: thread-safe in-memory store for testing - JSONLWriter: production-ready JSON lines file appender - Cancelled calls are distinguished from errors - CallContext.User.Name used as agent identifier - Extensions tracked per call 7 tests pass.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two new packages: auditlogger, which provides a server-side call interceptor for recording structured audit events to memory or JSONL files, and costlimiter, which implements cost-based budget enforcement for request handling. The review feedback highlights several critical improvements: resolving a memory leak in InMemoryCostStore by not storing unlimited scopes, using errors.Is to properly detect wrapped cancellation errors, optimizing JSONLWriter performance by moving JSON serialization outside of the critical section, and removing a redundant, non-atomic budget availability check before reservation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| v, ok := s.budget[scope] | ||
| if !ok { | ||
| s.budget[scope] = -cost | ||
| return true, nil | ||
| } |
There was a problem hiding this comment.
When a scope is not present in the budget map, it is considered unlimited. However, storing s.budget[scope] = -cost in the map for every new unlimited scope will cause the map to grow indefinitely over time (e.g., with unique user or agent IDs), leading to a memory leak. Since unlimited scopes do not need to be tracked, simply return true, nil without modifying the map.
v, ok := s.budget[scope]
if !ok {
return true, nil
}There was a problem hiding this comment.
Done. Reserve() now returns true, nil for unlimited scopes without modifying the map — no more memory leak.
|
|
||
| if resp != nil { | ||
| if resp.Err != nil { | ||
| if resp.Err == context.Canceled { |
There was a problem hiding this comment.
Use errors.Is instead of direct comparison (==) to check for context.Canceled. In Go, errors can be wrapped (e.g., via fmt.Errorf), and direct comparison will fail to detect wrapped cancellation errors. Note that you will need to import the errors package.
| if resp.Err == context.Canceled { | |
| if errors.Is(resp.Err, context.Canceled) { |
There was a problem hiding this comment.
Done. Changed to errors.Is(resp.Err, context.Canceled) for proper wrapped-error detection.
| func (w *JSONLWriter) Write(_ context.Context, ev *AuditEvent) error { | ||
| w.mu.Lock() | ||
| defer w.mu.Unlock() | ||
| b, err := json.Marshal(ev) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| b = append(b, '\n') | ||
| _, err = w.file.Write(b) | ||
| return err | ||
| } |
There was a problem hiding this comment.
Performing json.Marshal inside the critical section (while holding w.mu) can become a major performance bottleneck under high concurrent load, as JSON serialization is relatively CPU-intensive. Move json.Marshal outside of the lock so that only the actual file write operation is serialized.
| func (w *JSONLWriter) Write(_ context.Context, ev *AuditEvent) error { | |
| w.mu.Lock() | |
| defer w.mu.Unlock() | |
| b, err := json.Marshal(ev) | |
| if err != nil { | |
| return err | |
| } | |
| b = append(b, '\n') | |
| _, err = w.file.Write(b) | |
| return err | |
| } | |
| func (w *JSONLWriter) Write(_ context.Context, ev *AuditEvent) error { | |
| b, err := json.Marshal(ev) | |
| if err != nil { | |
| return err | |
| } | |
| b = append(b, '\n') | |
| w.mu.Lock() | |
| defer w.mu.Unlock() | |
| _, err = w.file.Write(b) | |
| return err | |
| } |
There was a problem hiding this comment.
Done. Moved json.Marshal outside the critical section — only file.Write holds the lock now.
| available, err := cl.store.Available(ctx, scope) | ||
| if err != nil { | ||
| return ctx, nil, fmt.Errorf("costlimiter: checking budget: %w", err) | ||
| } | ||
| if available >= 0 && available < cost { | ||
| return ctx, nil, fmt.Errorf("%w: scope %q: need %d, available %d", | ||
| ErrBudgetExceeded, scope, cost, available) | ||
| } | ||
|
|
||
| ok, err := cl.store.Reserve(ctx, scope, cost) | ||
| if err != nil { | ||
| return ctx, nil, fmt.Errorf("costlimiter: reserving budget: %w", err) | ||
| } | ||
| if !ok { | ||
| return ctx, nil, fmt.Errorf("%w: scope %q: budget exhausted", ErrBudgetExceeded, scope) | ||
| } |
There was a problem hiding this comment.
Calling cl.store.Available before cl.store.Reserve is redundant and introduces a non-atomic check-then-act window. Since Reserve is already responsible for atomically checking and deducting the budget, calling Available first results in an extra round-trip to the store (which could be a database or Redis in production) and doubles lock contention on the store's mutex. You can safely remove the Available check and call Reserve directly.
ok, err := cl.store.Reserve(ctx, scope, cost)
if err != nil {
return ctx, nil, fmt.Errorf("costlimiter: reserving budget: %w", err)
}
if !ok {
return ctx, nil, fmt.Errorf("%w: scope %q: budget exhausted", ErrBudgetExceeded, scope)
}There was a problem hiding this comment.
Done. Removed the redundant Available() check. Reserve() is already atomic — the check-then-act window is eliminated.
costlimiter.go: - Import limiter package for AttachScope/ScopeFrom - Remove default scopeFn initialization (leave nil), add resolveScope() - Before(): use resolveScope(), remove redundant Available() before Reserve() - After(): use context.WithoutCancel() for Release on cancelled contexts - InMemoryCostStore.Reserve(): don't store unlimited scopes in budget map costlimiter_test.go: - Import limiter package - Add TestCostLimiter_FallbackToLimiterScope - Add TestInMemoryCostStore_UnlimitedScopeNotStored auditlogger.go: - Add errors import - Use errors.Is(resp.Err, context.Canceled) instead of == comparison - Move json.Marshal outside mutex lock in JSONLWriter.Write()
|
Thanks @kuangmi-bit. The motivation isn't accurate. The existing log/ package is a wrapper around log/slog (Go's structured logging stdlib), not unstructured logging. And a2asrv.NewLoggingInterceptor (a2asrv/logging.go) is already a CallInterceptor that records method + duration + error on every A2A call using structured slog attributes, with a configurable level and optional payload capture. It covers the bulk of what this PR proposes. |
|
Thanks for the catch — you are right. The PR description called the output of Logging (
Auditing (the proposed
The distinction matters when compliance asks "show me every A2A call, with tenant and agent identity, in a tamper-proof format, for the last 90 days" — that is a fundamentally different ask than "what is my server doing right now." All that said — if the maintainers feel this does not warrant a separate package given the API surface concern (and for consistency with how #377 and #378 were handled), I am happy to close. Just wanted to clarify the actual motivation first, since the original description did a poor job of it. Appreciate the review. |
3 participants
Summary
Add
a2asrv/auditlogger— a server-sideCallInterceptorthat records structured audit events for every A2A protocol method invocation.Motivation
The existing
log/package provides unstructured logging. Enterprise deployments need structured, machine-readable audit trails for compliance, monitoring, and debugging. The audit event schema aligns with IBM SMF Type 110 (CICS transaction audit): who called what method, when, with what result, and at what duration.Design
AuditLogger: aCallInterceptor.Before()records start time;After()builds and writes the event.AuditWriter: pluggable interface. Ships with:InMemoryWriter: for tests and devJSONLWriter: production-ready, concurrent-safe JSON lines filesuccess,error, andcancelledresultsCallContext.User.Nameused as agent identifierExample
Testing
7 tests pass: success/error/cancelled result types, multiple events, no-user edge case, JSONL file writer, concurrent writes (200 events across 10 goroutines).